Repository navigation
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📝 WalkthroughWalkthroughThe webhook documentation now describes required secret keys, reserved custom-header names, and Standard Webhooks verification for Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The documentation presents consistent verification and key-rotation guidance. No actionable merge-blocking defect was found; publication should remain coordinated with the Standard Webhooks release as requested. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
@coderabbitai pause |
✅ Action performedReviews paused. |
|
Requesting review, but it is not urgent (this isn't ready to merge; I'm preparing it in advance) |
Albinutte
left a comment
Albinutte
left a comment
There was a problem hiding this comment.
Thanks for the update! Approving with a few suggestions 🙏
| overwrite it with a custom one. | ||
| We will generate a <strong>Secret key</strong> automatically. You can replace it | ||
| with a custom one, but we recommend keeping the generated key (see | ||
| <a href="#authkeyformat">Secret key format</a>). |
There was a problem hiding this comment.
Probably pre-existing, but it doesn't look great together with the image when the text wraps:
Separately, it would be great to update the screenshots -- they are very outdated 🙈
There was a problem hiding this comment.
I'll update the image layout to display it block instead of inline, thanks! I agree with you about the ancient screenshots for sure, but the scope is a little extra so I'm going to leave that out of this PR for now (all of the screenshots are ancient, but the docs overall needs a refresh and I'm not sure I want to get into it in scope of the Standard Webhooks update)
| you've already processed. If several of your webhooks receive the same event, | ||
| they all send the same <span class="mono">webhook-id</span>. Treat it as an |
There was a problem hiding this comment.
If several of your webhooks receive the same event, they all send the same
webhook-id
This sounds a bit confusing to me, but I can't really explain why 😅 Maybe rephrasing it as "If several of your webhooks receive the same event, each of them gets the same webhook-id." will be clearer?
There was a problem hiding this comment.
You're right. We're saying that webhooks both send and receive events. I think the root confusion is that "webhook" can be used to refer to "the registration that maps some events and headers to a particular handler endpoint" (which is normally how we use it), or it could be used to refer to the handler itself. In this sentence, it's sort of doing both at the same time, and that's strange.
Since our docs generally use meaning 1, I've updated this sentence to be more consistent with that interpretation.
This branch has not been deployed
2 participants
Warning
This is a public repo. Keep internal or private context, such as customer names, internal links, or implementation details, out of this PR and its comments. Discuss those in a private channel instead.
Hold: Don't merge until Standard Webhooks signatures are released.
Changes
Updates the webhook docs for the new signature headers:
webhook-id,webhook-timestamp, andwebhook-signatureheaders, with examples for verifyingwebhook-signatureusing an official Standard Webhooks library or by hand.whsec_secret key format, and that keys in the older format don't receivewebhook-signatureuntil they're regenerated.X-CodeSignal-Signatureas legacy. It's still sent, so existing integrations keep working.webhook-idand get a newwebhook-timestamp.Both code samples were tested against signatures generated the same way the server generates them.
Testing
You can run the site locally with
npm start. Browse the new authentication section in our webhook docs and verify that it matches our updated expectations.